Regression fixes and prepare 0.2.0#121
Conversation
Since our on_event was skipping the inner filter, the disabled bits would stay set for the next event, which coincidentally could end up being an event logged with `gitlab.output=true` directly. This seems to not have come up in testing because the events that got accidentally disabled were TRACE events that we didn't really care about anyway. The tests are also modified to only enable TRACE-level logging when OBO_TEST_TRACE is set, that way the tests are doing logging with a setup more similar to the runner proper.
Since the CLI trace logs end up in the same place as the output logs, we need a small hack to differentiate them in one test, but it's only enabled when test tracing is enabled and thus shouldn't cause problems elsewhere.
gitlab-runner 0.0.8+ adds support for artifact filtering, which we were preparing for here...but the current syntax does not actually match GitLab's own syntax and instead treats the patterns as globs for individual files (not directories). Thus, we need to turn the results directory name into a glob in order for them to actually be uploaded as artifacts.
refi64
force-pushed
the
wip/refi64/prep-0.2.0
branch
2 times, most recently
from
3ee5231 to
6f6123e
Compare
| @@ -73,12 +73,32 @@ impl<S: Subscriber + Send + Sync + 'static + for<'span> LookupSpan<'span>, F: Fi | |||
| } | |||
|
|
|||
| fn on_event(&self, event: &Event<'_>, ctx: Context<'_, S>) { | |||
There was a problem hiding this comment.
Can you remind me why obs-gitlab-runner has it's own logging handling in the first place? Also there a similar problem in gitlab-runner-rs ?
There was a problem hiding this comment.
It's because, as part of having both the CLI and runner backed by the same core logic, we needed a way for the core logic to go "hey log a message for the user" in generalized way, and the options were basically:
- wire up some custom generalized logging interface ("we have tracing at home").
- just use tracing like gitlab-runner does, which is nice to use, but does mean we have to do some iffy logic to map from our custom generalized logging field to the field the runner actually wants.
There was a problem hiding this comment.
You didn't answer the second part and arguably more important part of the comment. Does gitlab-runner-rs need a similar fix?
There was a problem hiding this comment.
the core deciding which message go the user is typically an antipattern though; but that's not really something for this PR :p
| derivative = "2.2" | ||
| futures-util = "0.3" | ||
| open-build-service-api = "0.1.0" | ||
| open-build-service-api = { git = "https://github.com/collabora/open-build-service-rs" } |
There was a problem hiding this comment.
Create a new release instead, don't use git versions
There was a problem hiding this comment.
I mentioned in the PR body
If collabora/open-build-service-rs#65 is merged first then I can change this to point to the new version instead of git.
Otherwise, generated pipelines might end up with bad `artifacts` items without it being caught during testing.
This contains fixes for logs being blank in some GitLab versions: collabora/gitlab-runner-rs#125 https://docs.gitlab.com/releases/patches/patch-release-gitlab-19-0-2-released/ Fixes #119.
This contains a fix for infinite looping when downloading the logs when reqwest uses HTTP/2.
This makes it easier to bump the version for the CLI + runner and reduces duplication.
refi64
force-pushed
the
wip/refi64/prep-0.2.0
branch
from
6f6123e to
47aae60
Compare
2 participants
Contains the gitlab-runner upgrade to work with newer GitLab, plus three regression fixes for the runner:
generate-pipelineto use syntax compatible with the current gitlab-runner version.The latter two should now be caught via the test suite, and the first should be caught by the newly added, vaguely horrifying smoke test bash script.